Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow ClientHello to span multiple QUIC packets #3045

Merged
merged 9 commits into from
Oct 31, 2019

Conversation

DavidSchinazi
Copy link
Contributor

The restriction for ClientHello to fit in a single UDP datagram was added back when the retry mechanism was more tightly coupled to the TLS crypto. At this point, the restriction doesn't get us much, and is preventing us from running some post-quantum experiments.

Fixes #2928.

to decide whether to accept the new incoming QUIC connection. If the
ClientHello spans multiple Initial packets, such servers would need to buffer
the first received fragments, which could consume excessive resources if the
client's address has not yet been validated. To avoid this, servers MAY use
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resources are consumed either way, but if the client's address has not yet been validated, a server might not be willing to commit those resources.

the Retry feature (see Section 8.1 of {{QUIC-TRANSPORT}}) to only buffer
partial ClientHellos from clients with a validated address. Though a packet
larger than 1200 bytes might be supported by the path, a client improves the
likelihood that a packet is accepted if it ensures that the first ClientHello
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/is accepted/isn't dropped while in transit/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that acceptance and loss are two separate things and what we're talking about here is acceptance. The predicate on the statement is that the path supports packets of that size. This is speculating about a server's willingness to buffer the ClientHello bytes.

To that end, this isn't about the 1200 byte minimum MTU, it's about making the ClientHello as acceptable as possible. Fragments are immediately less acceptable, and while smaller fragments are easier to hold, what matters is the size of the entire message, because that's the upper limit on what the server might have to buffer. Non-fragmented ClientHello can be as large as the path will support, because the server doesn't have to save them.

I think that we can strike this last sentence and things will be clearer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to "as acceptable as possible," should we try to recommend a minimum bound on fragments? (I'm reminded of data dribble, even though that's a different problem entirely.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this multiple packet initial thing lands, at least we should limit the packet count / time to arrival or provide guidance. Dribble (slow loris) can happen in fragments or just in many very small complete packets over a very long time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that servers are encouraged to send Retry packets to clients with multiple-packet ClientHellos, this change doesn't make dribble worse, an attacker can send a single-packet ClientHello then start its SlowLoris without this change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to commit as much memory after that point.

@nibanks
Copy link
Member

nibanks commented Sep 17, 2019

This proposed changes imposes some new, nontrivial requirements on the server side. As a server, would I be required to support multi-packet client initials? Do we really need this in V1?

@dtikhonov
Copy link
Member

I share @nibanks's concern: The pain in the ass would be quite real. Is post-quantum crypto in scope?

@kazuho
Copy link
Member

kazuho commented Sep 17, 2019

@nibanks

As a server, would I be required to support multi-packet client initials?

I'd point out that endpoints are already required to support muti-packet client Initials when the TLS handshake uses HRR. Therefore, IMO the PR does not add new threats.

@nibanks
Copy link
Member

nibanks commented Sep 17, 2019

HRR is not stateless (at the QUIC layer) though, correct? The client's first packet still must completely contain ALPN and (optionally) SNI. That information is used by WinQuic to decide initially if the connection should be accepted and more server resources commit to processing the connection. Adding another point to the QUIC layer to decide if Retry is needed is additional work.

@kazuho
Copy link
Member

kazuho commented Sep 17, 2019

@nibanks I might argue that this PR does not prevent you from using the ALPN and SNI in the first packet as a way to "skip" Retry. In that hack, the difference between the status quo and this PR is that an incomplete CH in the first packet becomes a signal to send a Retry rather than an error. I do not think it adds complexity.

Considering the fact that we can remove other complexities due to the 1-packet restriction, this PR is a good simplification.

@nibanks
Copy link
Member

nibanks commented Sep 17, 2019

I'm not arguing that this PR would require loads more code, but I definitely don't agree that it's a "good simplification" of the current design. This adds additional attack surfaces that must be analyzed and implementations ensure they have adequate defense.

Again, I ask is this absolutely required for V1? What chartered requirements for QUIC deem this change necessary for V1?

@kazuho
Copy link
Member

kazuho commented Sep 17, 2019

At the moment, a QUIC stack is required to indicate to the TLS stack that the CH should be constructed in a way that invokes HRR (by omitting the key_share extension), if the CH is otherwise going to not fit in one packet. This requirement is not only complex but also imposes one additional roundtrip (when the rule is invoked).

Regarding the charter, we are tasked to create a protocol that minimizes connection establishment latency, while having "security and privacy properties that are at least as good as a stack composed of TLS 1.3 using TCP." We already have post-quantum extensions proposed for TLS 1.3, it is my understanding that Chrome is already doing experiments. They require use of CH that do not fit in single packet, which, without resolving this issue, imposes complexity and one extra round-trip. Considering these aspects, I might argue that we are encouraged by the charter to resolve the problem :-)

@ianswett ianswett added the -tls label Sep 17, 2019
Copy link
Member

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is text in the transport draft that needs updating as well.

the Retry feature (see Section 8.1 of {{QUIC-TRANSPORT}}) to only buffer
partial ClientHellos from clients with a validated address. Though a packet
larger than 1200 bytes might be supported by the path, a client improves the
likelihood that a packet is accepted if it ensures that the first ClientHello
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that acceptance and loss are two separate things and what we're talking about here is acceptance. The predicate on the statement is that the path supports packets of that size. This is speculating about a server's willingness to buffer the ClientHello bytes.

To that end, this isn't about the 1200 byte minimum MTU, it's about making the ClientHello as acceptable as possible. Fragments are immediately less acceptable, and while smaller fragments are easier to hold, what matters is the size of the entire message, because that's the upper limit on what the server might have to buffer. Non-fragmented ClientHello can be as large as the path will support, because the server doesn't have to save them.

I think that we can strike this last sentence and things will be clearer.

draft-ietf-quic-tls.md Outdated Show resolved Hide resolved
@mikkelfj
Copy link
Contributor

Regardless of post-quantum and HRR, I agree with @nibanks that it is problematic to commit multiple packets before any kind of validation. If that is already the case due to HRR, I hope there is a way to avoid that without relying on client generated content. Otherwise the server is wide open to slow loris connection attacks.

As to post-quantum - fine to have an experimental setting - in a real post-quantum world, perhaps min MTU can be larger?

Copy link
Member

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some residual singular/plural disagreements.

draft-ietf-quic-transport.md Outdated Show resolved Hide resolved
draft-ietf-quic-transport.md Outdated Show resolved Hide resolved
draft-ietf-quic-transport.md Outdated Show resolved Hide resolved
DavidSchinazi and others added 2 commits September 24, 2019 17:15
Co-Authored-By: Martin Thomson <mt@lowentropy.net>
Copy link
Contributor

@ianswett ianswett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor nits, but the core content looks good.

draft-ietf-quic-tls.md Outdated Show resolved Hide resolved
draft-ietf-quic-tls.md Outdated Show resolved Hide resolved
draft-ietf-quic-transport.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@ekr ekr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Note that if the server sends a HelloRetryRequest, the client will send another
series of Initial packets. These Initial packets will continue the
cryptographic handshake and will contain CRYPTO frames starting at an offset
matching the size of the CRYPTO frames sent in the first flight of Initial
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"matching the size" is a bit odd. Perhaps "immediately after the last byte of..."

Copy link
Contributor

@MikeBishop MikeBishop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The text of this PR looks good. I share the concern about whether we want to support multi-packet ClientHello messages, though the discussion (which should be on the issue, not the PR) seems fairly convincing that we'll need to do this at some point if we don't do it now.

If the ClientHello spans multiple Initial packets, such servers would need to
buffer the first received fragments, which could consume excessive resources if
the client's address has not yet been validated. To avoid this, servers MAY
use the Retry feature (see Section 8.1 of {{QUIC-TRANSPORT}}) to only buffer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or address tokens, presumably?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lift single-packet ClientHello requirement?